New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[full-ci] Implement people sharing for spaces #6455

Merged

kulmann merged 8 commits into master from spaces-collaborators

Mar 7, 2022

Contributor

JammingBen commented Feb 18, 2022 •

edited

Loading

Description

Spaces can now be shared with other people. This change specifically includes:

listing all members who have access to a space (possible for all space members)
adding members to a space and giving them dedicated roles (possible for managers only)
editing the role of members (possible for managers only)
removing members from a space (possible for managers only)

~~Needs owncloud/owncloud-sdk#1013 to work properly.~~ -> I've created a new alpha version for the SDK and updated the dependency in this PR. So running yarn is all you need to do.

Note that this PR only tackles the sharing of spaces via people. It does not include:

Link sharing
Group sharing
Sharing of resources within a space

Known issues

Querying each user via the graph API is really bad performance-wise. Needs Expand the drive GET-endpoints with user information ocis#3195 to be implemented first though
The permissions method of each role takes allowSharing as argument, because this permission is not determined by the roles, but by the given backend (?). However, with spaces, this changes. I've overcome this for now by always passing true to the method when dealing with shares... but there might be better solutions.
The displayed collaborators on the space front page might be wrong due to backend caching issues. The spaces overview works fine however.

Related Issue

Make clear who else has access to a Space #6283

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests

Checklist:

Code changes
Unit tests added
Acceptance tests added
Documentation ticket raised:

JammingBen self-assigned this

JammingBen changed the title ~~Implement people sharing for spaces~~ [WIP] Implement people sharing for spaces

delete-merged-branch bot deleted the branch master

February 19, 2022 00:41

JammingBen changed the base branch from spaces-sidebar to master

February 28, 2022 09:39

JammingBen force-pushed the spaces-collaborators branch from 1c71bb8 to 365225c Compare

February 28, 2022 09:39

JammingBen mentioned this pull request

Display Space properties in sidebar info-panel #6284

Closed

8 tasks

Contributor

ownclouders commented Feb 28, 2022

Results for oC10SharingIntGroupsToRoot https://drone.owncloud.com/owncloud/web/23100/28/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUISharingInternalGroupsToRootEdgeCases-shareWithGroupsEdgeCases_feature-L36.png

💥 The acceptance tests pipeline failed. The build has been cancelled.

Contributor

ownclouders commented Feb 28, 2022

Results for oC10SharingIntGroupsToRoot https://drone.owncloud.com/owncloud/web/23109/28/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUISharingInternalGroupsToRootEdgeCases-shareWithGroupsEdgeCases_feature-L36.png

💥 The acceptance tests pipeline failed. The build has been cancelled.

Contributor

ownclouders commented Feb 28, 2022

Results for oC10SharingIntGroupsToRoot https://drone.owncloud.com/owncloud/web/23114/28/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUISharingInternalGroupsToRootEdgeCases-shareWithGroupsEdgeCases_feature-L36.png

💥 The acceptance tests pipeline failed. The build has been cancelled.

JammingBen force-pushed the spaces-collaborators branch from 64546a0 to cd90c58 Compare

March 1, 2022 11:20

Contributor

ownclouders commented Mar 1, 2022

Results for oC10SharingIntGroupsToRoot https://drone.owncloud.com/owncloud/web/23133/28/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUISharingInternalGroupsToRootEdgeCases-shareWithGroupsEdgeCases_feature-L36.png

💥 The acceptance tests pipeline failed. The build has been cancelled.

JammingBen changed the title ~~[WIP] Implement people sharing for spaces~~ [WIP][full-ci] Implement people sharing for spaces

Contributor

ownclouders commented Mar 1, 2022

Results for oC10SharingIntGroupsToRoot https://drone.owncloud.com/owncloud/web/23139/28/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUISharingInternalGroupsToRootEdgeCases-shareWithGroupsEdgeCases_feature-L36.png

Contributor

ownclouders commented Mar 1, 2022

Results for oC10SharingIntGroups https://drone.owncloud.com/owncloud/web/23139/27/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUISharingInternalGroupsEdgeCases-shareWithGroupsEdgeCases_feature-L41.png

Contributor

ownclouders commented Mar 1, 2022

Results for e2e-tests oCIS https://drone.owncloud.com/owncloud/web/23145/11/1

💥 To see the trace, please open the link in the console ...

npx playwright show-trace https://cache.owncloud.com/public/owncloud/web/23145/tracing/alice-can-share-this-weeks-meal-plan-with-all-parents-alice-2022-3-1-02-01-42.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/web/23145/tracing/alice-shares-file-to-brian-alice-2022-3-1-02-03-03.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/web/23145/tracing/alice-shares-file-to-brian-brian-2022-3-1-02-03-09.zip

Contributor

ownclouders commented Mar 1, 2022

Results for oCISSharingInternal3 https://drone.owncloud.com/owncloud/web/23146/61/1
The following scenarios passed on retry:

webUISharingInternalUsersShareWithPage/shareWithUsers.feature:91

Contributor

ownclouders commented Mar 1, 2022

Results for oCISSharingAndUpload https://drone.owncloud.com/owncloud/web/23146/66/1
The following scenarios passed on retry:

webUIUpload/upload.feature:69

JammingBen force-pushed the spaces-collaborators branch from bbf4a57 to d0717e9 Compare

March 2, 2022 09:17

JammingBen changed the title ~~[WIP][full-ci] Implement people sharing for spaces~~ [full-ci] Implement people sharing for spaces

JammingBen marked this pull request as ready for review

March 2, 2022 09:24

JammingBen requested review from pascalwengerter and fschade as code owners

March 2, 2022 09:24

JammingBen requested review from AlexAndBear and kulmann

March 2, 2022 09:24

Contributor

ownclouders commented Mar 2, 2022

Results for oCISFiles1 https://drone.owncloud.com/owncloud/web/23167/55/1
The following scenarios passed on retry:

webUIDeleteFilesFolders/deleteFilesFolders.feature:77
webUIDeleteFilesFolders/deleteFilesFolders.feature:253

kulmann requested changes

View reviewed changes

Member

kulmann left a comment

From a first try in the web ui, I feel like talking about members instead of people would be more appropriate. Meaning:
1: Members as title for the right sidebar panel
2: 1 member, 7 members in the link after the space title in the space root
Please discuss with @tbsbdr and @pmaier1

One more tiny detail: I think it would be more intuitive to have the three dots button for the spaces context menu after the title and only after that the members info. Looks a bit off, that the members info is in between the title and the three dots button.

When I try to share a space I get the following error message in the browser console (and nothing happens regarding sharing):

vendor-1671c65f.js:11 TypeError: i.shares.shareSpaceWithUser is not a function
    at rx.addShare (web-app-files-d3f6f72f.js:1:209447)
    at Array.<anonymous> (vendor-1671c65f.js:37:19210)
    at rx.dispatch (vendor-1671c65f.js:37:20947)
    at rx.dispatch (vendor-1671c65f.js:37:16415)
    at rx.i.dispatch (vendor-1671c65f.js:37:18321)
    at a.gx.forEach.r.<computed> (vendor-1671c65f.js:37:23607)
    at web-app-files-d3f6f72f.js:1:258071
    at vendor-1671c65f.js:37:33140
    at Rx._tryToStartAnother (vendor-1671c65f.js:37:32187)
    at vendor-1671c65f.js:37:33349

Contributor

tbsbdr commented Mar 2, 2022

member/s here ... and here:

Contributor Author

JammingBen commented Mar 2, 2022

When I try to share a space I get the following error message in the browser console (and nothing happens regarding sharing)

Sounds like you didn't link owncloud/owncloud-sdk#1013. My plan is to create a new alpha release once the PR is merged and add it here. But I'm still waiting for feedback from the OCIS team.

Contributor

tbsbdr commented Mar 2, 2022 •

edited

Loading

Proposal to make the Headline-3Dots-Memberscount-line more appealing

Full width of the description
3-dots next to Space-Title
Members Count aligned to the left
Space-image: ratio 16:9 - height is fixed

Please speak up, if you don't agree

Contributor

tbsbdr commented Mar 2, 2022 •

edited

Loading

updated screenshot above: Members-count has the icon now on the left.

JammingBen force-pushed the spaces-collaborators branch from 2cc64d5 to 16326e4 Compare

March 3, 2022 10:25

JammingBen requested a review from kulmann

March 3, 2022 10:32

JammingBen force-pushed the spaces-collaborators branch from 16326e4 to 7da85df Compare

March 4, 2022 12:07

kulmann requested changes

View reviewed changes

Member

kulmann left a comment

Please don't feel intimidated by the number of change requests - they should be easily resolved. This PR is in really good shape and a huge step forward. Awesome! 🥳

packages/web-app-files/src/components/SideBar/Shares/InviteCollaborator/RecipientContainer.vue Outdated Show resolved Hide resolved

packages/web-app-files/src/components/SideBar/Details/SpaceDetails.vue Outdated Show resolved Hide resolved

packages/web-app-files/src/components/SideBar/Details/SpaceDetails.vue Show resolved Hide resolved

...es/web-app-files/src/components/SideBar/Shares/InviteCollaborator/InviteCollaboratorForm.vue Outdated Show resolved Hide resolved

packages/web-app-files/src/helpers/share/role.ts Outdated Show resolved Hide resolved

packages/web-app-files/src/components/SideBar/Shares/SpaceShares.vue Outdated Show resolved Hide resolved

packages/web-app-files/src/components/SideBar/Shares/SpaceShares.vue Show resolved Hide resolved

packages/web-app-files/src/components/SideBar/Shares/SpaceShares.vue Outdated Show resolved Hide resolved

packages/web-app-files/src/views/spaces/Project.vue Show resolved Hide resolved

packages/web-app-files/src/views/spaces/Projects.vue

@@ @@ -361,6 +378,11 @@ export default { @@
                       return 'state-trashed'
                     }
                     return ''
+                  },
+                  async openSidebarSharePanel(space) {
+                    await this.closeSidebar()

Member

kulmann Mar 4, 2022

Why is it needed to close the sidebar?

Contributor Author

JammingBen Mar 4, 2022

Don't know why I added it, it's not :D

Contributor Author

JammingBen Mar 7, 2022

Ahh, I found the issue again, it is. It seems to be necessary when toggling through the share panel in the spaces overview (via the people-indicator). Otherwise, the share panel does not get updated.

Member

kulmann Mar 7, 2022

Isn't that an indicator for data not properly watched in the sharing panel? Can you look into that after merge? Closing and re-opening the sidebar feels a bit glitchy and might cause side effects in the future.

Member

kulmann commented Mar 4, 2022 •

edited

Loading

owncloud-sdk v2.1.0-alpha.3 has been merged to master in the meantime. Instead of rebasing, it should be sufficient to cherry-pick the commit e36e14711fcd5b758ac998c484d121d11eaff4b2 into your branch.

JammingBen added 8 commits

March 7, 2022 09:35


          Implement people sharing for spaces

b8fd829


          Add and adjust unit tests

e9428dd


          Add changelog item

8c891b3


          Use uuid instead of index for the role dropdown class

de908c8


          Redesign the project view a bit

0db2646


          Rename 'people' to 'members' for spaces

bad70e7


          Minor adjustments according to code review

18577d3


          Update snapshot

7a4b939

JammingBen force-pushed the spaces-collaborators branch from 7da85df to 7a4b939 Compare

March 7, 2022 08:51

sonarcloud bot commented Mar 7, 2022

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

72.9% Coverage
0.0% Duplication

JammingBen requested a review from kulmann

March 7, 2022 09:01

kulmann approved these changes

View reviewed changes

Member

kulmann left a comment

Great job! 😍

packages/web-app-files/src/components/SideBar/Shares/RoleDropdown.vue Show resolved Hide resolved

packages/web-app-files/src/views/spaces/Projects.vue

@@ @@ -361,6 +378,11 @@ export default { @@
                       return 'state-trashed'
                     }
                     return ''
+                  },
+                  async openSidebarSharePanel(space) {
+                    await this.closeSidebar()

Member

kulmann Mar 7, 2022

Isn't that an indicator for data not properly watched in the sharing panel? Can you look into that after merge? Closing and re-opening the sidebar feels a bit glitchy and might cause side effects in the future.

kulmann merged commit eb10385 into master

delete-merged-branch bot deleted the spaces-collaborators branch

March 7, 2022 11:03

JammingBen mentioned this pull request

Add a watcher for the share panel of a space #6543

Merged

9 tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet